fix(data): sort 的点号路径被拒绝(400 INVALID_SORT),不再静默降级为不排序 (#4256) - #4303
Merged
Conversation
The one sort shape #4226 deliberately left open. `?sort=account.company_name` passed the gate on its head segment (a real field) and was then unusable by every driver: SqlDriver rendered `"account"."company_name"` against a table that was never joined and the #3821 backstop retried WITHOUT the sort; Mongo and the memory driver resolved the path against the row, where a foreign key is a scalar id. 200, every row present, arbitrary order — which `top` then sliced into an arbitrary "latest N". Now `400 INVALID_SORT` at the shared normalizer, with the message split by what the head segment is: a relationship head names the relation it tried to cross and prescribes denormalising the value onto the queried object (formula / rollup field); a non-reference head states the contract (sort reaches whole columns, not values inside them). Unknown heads keep the #4226 typo answer, reported first — the same precedence the expand gate uses. A survey of framework, objectui and cloud found zero callers emitting a dotted sort; objectui's column-header sort structurally cannot produce one (lookup columns are keyed by their flat field name, relations load via $expand). Blast radius is hand-authored requests — exactly the callers the silent degradation was misleading. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Aug 1, 2026
Closed
os-zhuang
pushed a commit
that referenced
this pull request
Aug 1, 2026
…ECORD_NOT_FOUND, not 200 (#4435) The READ path was already honest — `getData` on an unknown id answers `404 RECORD_NOT_FOUND`. Both single-record WRITE paths reported success for a record that does not exist: PATCH /data/showcase_task/definitely_not_a_row → 200 {"record":null} DELETE /data/showcase_task/definitely_not_a_row → 200 {"success":true} REST is a pass-through here (`res.json(await p.deleteData(...))`), so these are the protocol's answers and this is where they are fixed. What it cost: a client that PATCHed a concurrently deleted record was told the write landed, and had to null-check a SUCCESS payload to find out otherwise; `DELETE` said `success: true` for any string in the path, so a typo'd id, an already-deleted row and a real deletion were indistinguishable — including in bulk, where `deleteMany {"ids":["nonexistent_1"]}` answered `succeeded: 1`. It is the same silent-no-op shape the v17 train removed everywhere else this window (#4240/#4303/#4315, #4169, #4190), one level up. - `updateData` asks existence BEFORE the write, via the same `findOne` + caller context `getData` uses. Deliberately not a post-check on the returned row: the engine returns the post-write READBACK, which is also `null` when the row still exists but the write moved it out of the caller's row scope (reassigning `owner_id` away from yourself under an owner-scoped policy) — reading that as "not found" would 404 a write that succeeded. - `deleteData` and `deleteManyData` read the driver's own answer. The contract (`IDataDriver.delete` — "True if deleted, false if not found") already carried it; the code discarded it and pushed a literal `success: true`. Read as `=== false` on purpose: that is the contract's positive not-found value, while a driver returning the deleted row or an off-contract `undefined` gives no such signal, and inventing a 404 from a falsy return would break deletes against third-party drivers instead of reporting honestly. `success` on the 200 now means what it says. - The 404 envelope is extracted as `recordNotFoundError` so the read and the two write paths cannot drift apart again. Note on the issue's second half: the spec's `DeleteDataResponseSchema` declares `success`, not `deleted`, so the existing key is correct as-is and nothing renames. Tests: new `protocol.record-not-found.test.ts` (12) covers PATCH/DELETE/ deleteMany, the read/write agreement on the same id, delete-twice, mixed batches, the `=== false` reading, and that the existence probe is asked with the caller's context. Three `protocol.dropped-fields.test.ts` fixtures stubbed `findOne → null` while PATCHing — under the new contract that IS a 404, so they now describe an engine that has the row (they are about the strip channel, not about missing records). Suites green: metadata-protocol 169, rest + objectql unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 2, 2026
…bjectstack-ai#4435, objectstack-ai#4436, objectstack-ai#4483) (objectstack-ai#4496) * fix(spec): the $search auto field set's lead ORDERS the set, it must not admit one (objectstack-ai#4483) `autoDefaultFields` filtered every field through three exclusions (`SEARCH_AUTO_EXCLUDED_FIELDS`, `hidden`, unsearchable type) and then prepended the display/name/title field on an EXISTENCE check alone — so the exclusions did not hold for whichever field happened to lead, and the module's own "system / audit / heavy fields never auto-included" invariant was false. Not a contrived shape: ADR-0079's `provisionPrimary(schema, { synthesize: false })` designates `nameField` at registration, and on a table whose only textual column IS the primary key (system tables, junction tables, append-only logs) it designates `id`. `$search` then expanded to `{ id: { $contains: term } }` — a substring scan over the primary key, returning a narrow and semantically wrong row set. It loosened a second layer too: `resolveSearchFieldResolution` is also the objectstack-ai#4254 REST ingress gate's arbiter for "would the engine actually scan this field", so with `id` in `allowed` a `$searchFields=id` override was ACCEPTED rather than refused. The lead's job is to put the primary title FIRST, never to admit it, so it is now chosen from the already-filtered set. An excluded / hidden / unsearchable display field simply does not lead and the set is unchanged; an eligible one still leads, so the ordering intent is intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * wip(drivers): give the uncompilable-filter refusal an ADR-0112 code and drop the driver prefix (objectstack-ai#4436) IN PROGRESS — code change complete, regression test not yet written and the real-boot curl repro not yet run. A filter carrying an operator the driver cannot compile is already REFUSED rather than silently matched (objectstack-ai#4209/objectstack-ai#4029), but the refusal had no wire identity: the thrown `Error` carried no `code`, so `mapDataError`'s default branch served `{"error": "[sql-driver] Unsupported filter operator …"}` — no `error.code` at all, breaking the ADR-0112 contract every sibling rejection on the same route already honours (`INVALID_FIELD`, `INVALID_FILTER`, `RECORD_NOT_FOUND`), and leaking the `[sql-driver]` internal prefix that the objectstack-ai#3867 sanitiser exists to keep off the wire. Both drivers now throw through a local `unsupportedFilterError` that stamps `code = StandardErrorCode.enum.INVALID_FILTER` (the same catalogued code `metadata-protocol` emits when a filter fails to parse upstream — one condition, one wire code however the caller reached it) and `status = 400`, which also puts the rejection on `isExpectedQueryRejection` so a client mistake stops being logged as an unhandled server error. The internal prefix is gone from the message; the actionable operator/field/vocabulary detail stays. Applied to every filter-COMPILATION refusal in both backends, not just the one branch the issue names — they are the same envelope defect on adjacent lines, and objectstack-ai#3948 made the two drivers agree that an uncompilable filter is a refusal, so their refusal envelopes have to agree too. TODO: regression tests (driver-sql, driver-memory, REST envelope) + boot repro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(drivers): the uncompilable-filter refusal speaks INVALID_FILTER, without the driver prefix (objectstack-ai#4436) Completes the WIP commit: adds the remaining sql-driver throw sites and the regression tests for both backends. objectstack-ai#4209/objectstack-ai#4029/objectstack-ai#3948 settled the POSTURE — a filter carrying an operator the driver cannot compile is refused instead of silently matching every row. What was missing is the refusal's IDENTITY on the wire. The driver threw a bare `Error`, so `mapDataError` fell through to its default branch and served a body whose only key was `error`: GET /api/v1/data/showcase_task?filter={"title":{"$bogusop":"x"}} → 400 {"error":"[sql-driver] Unsupported filter operator \"$bogusop\" …"} Two contract breaks in one body — no `error.code` at all on a route whose sibling rejections all speak the ADR-0112 catalogue, and the driver-internal `[sql-driver]` prefix on the wire, which is what the objectstack-ai#3867 sanitiser exists to stop. Fixed at the throw site (PD objectstack-ai#12), not by teaching the REST layer to guess: both drivers now refuse through an `unsupportedFilterError` helper that stamps `code = StandardErrorCode.enum.INVALID_FILTER` — the constant, so a catalogue rename breaks the compile — and `status = 400`. `INVALID_FILTER` is the same code `metadata-protocol` already emits when a filter fails to parse upstream (`malformedFilterArrayError` / `unusableFilterError`): one condition, one wire code, however the caller reached it. The `status` also puts the rejection on `isExpectedQueryRejection`, so a client mistake stops being logged as an unhandled server error. Applied to every filter-COMPILATION refusal in both backends, not only the one branch the issue names: unsupported operator ($-object, legacy triple), unrecognised logical keyword, unrecognised element type, and a `between` / `$between` operand that is not a two-element array. They are the same envelope defect on adjacent lines, and objectstack-ai#3948 made the two drivers agree that an uncompilable filter is a refusal — so their refusal envelopes have to agree too, or the cross-driver parity this repo relies on is false where it matters. Tests: new `sql-driver-filter-refusal-envelope.test.ts` (8) and `memory-filter-refusal-envelope.test.ts` (5) pin `code`, `status`, the absence of the internal prefix, and that the actionable operator/field/vocabulary detail survives. Full suites green: driver-sql 623 passed, driver-memory 286 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(metadata-protocol): PATCH/DELETE of a nonexistent record answer RECORD_NOT_FOUND, not 200 (objectstack-ai#4435) The READ path was already honest — `getData` on an unknown id answers `404 RECORD_NOT_FOUND`. Both single-record WRITE paths reported success for a record that does not exist: PATCH /data/showcase_task/definitely_not_a_row → 200 {"record":null} DELETE /data/showcase_task/definitely_not_a_row → 200 {"success":true} REST is a pass-through here (`res.json(await p.deleteData(...))`), so these are the protocol's answers and this is where they are fixed. What it cost: a client that PATCHed a concurrently deleted record was told the write landed, and had to null-check a SUCCESS payload to find out otherwise; `DELETE` said `success: true` for any string in the path, so a typo'd id, an already-deleted row and a real deletion were indistinguishable — including in bulk, where `deleteMany {"ids":["nonexistent_1"]}` answered `succeeded: 1`. It is the same silent-no-op shape the v17 train removed everywhere else this window (objectstack-ai#4240/objectstack-ai#4303/objectstack-ai#4315, objectstack-ai#4169, objectstack-ai#4190), one level up. - `updateData` asks existence BEFORE the write, via the same `findOne` + caller context `getData` uses. Deliberately not a post-check on the returned row: the engine returns the post-write READBACK, which is also `null` when the row still exists but the write moved it out of the caller's row scope (reassigning `owner_id` away from yourself under an owner-scoped policy) — reading that as "not found" would 404 a write that succeeded. - `deleteData` and `deleteManyData` read the driver's own answer. The contract (`IDataDriver.delete` — "True if deleted, false if not found") already carried it; the code discarded it and pushed a literal `success: true`. Read as `=== false` on purpose: that is the contract's positive not-found value, while a driver returning the deleted row or an off-contract `undefined` gives no such signal, and inventing a 404 from a falsy return would break deletes against third-party drivers instead of reporting honestly. `success` on the 200 now means what it says. - The 404 envelope is extracted as `recordNotFoundError` so the read and the two write paths cannot drift apart again. Note on the issue's second half: the spec's `DeleteDataResponseSchema` declares `success`, not `deleted`, so the existing key is correct as-is and nothing renames. Tests: new `protocol.record-not-found.test.ts` (12) covers PATCH/DELETE/ deleteMany, the read/write agreement on the same id, delete-twice, mixed batches, the `=== false` reading, and that the existence probe is asked with the caller's context. Three `protocol.dropped-fields.test.ts` fixtures stubbed `findOne → null` while PATCHing — under the new contract that IS a 404, so they now describe an engine that has the row (they are about the strip channel, not about missing records). Suites green: metadata-protocol 169, rest + objectql unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * fix(runtime): a sandbox capability denial is a 500 crash, not a 400 rejection (objectstack-ai#4431) The `action-crash-vs-rejection` contract (objectstack-ai#3951) pins the table: a `SandboxError` WITH `innerMessage` is a body's deliberate throw → 400; a `SandboxError` with NO `innerMessage` — timeout, capability denial — is a crash → 500. Capability denials were answering 400: POST /api/v1/actions/showcase_task/rc1_crash_probe → 400 {"error":{"code":"VALIDATION_ERROR", "message":"SandboxError: capability 'api.read' not granted to action …"}} Why: the gate throws `SandboxError` synchronously INSIDE a QuickJS host function, which rejects the async IIFE inside the VM, so it returns through the `__error` side-channel — and the pump loop presumed everything arriving there was user code throwing on purpose, setting `innerMessage` unconditionally. The dispatcher's classifier then read that as a deliberate rejection. So every capability denial stayed invisible to gateway error rates, APM and alerting — exactly the blindness objectstack-ai#3951 was written to close — and the client also received the `SandboxError: ` debug prefix that belongs only in server logs. `SandboxError`'s own jsdoc already said `innerMessage` is undefined for the sandbox's internal errors; that only held for denials detected OUTSIDE evaluation (a timeout, which takes the separate `budgetError` path). In-VM host-call denials — `ctx.api.*`, `ctx.log`, `ctx.crypto`, `ctx.api.transaction` — were misclassified. Fix: the sandbox's own faults now carry a marker THROUGH the VM. `hostErrorToVm` stamps `__objectstackSandboxFault` on any `SandboxError` it marshals, and the synchronous gates throw the VM handle it builds rather than a raw host error — quickjs-emscripten passes a thrown handle through verbatim while its `newError` path copies only `name`/`message`, which is precisely how the identity was lost. The reject handler reports the marker on the additive `__errorInfo` channel, and the pump loop, seeing it, rethrows with neither the `<kind> '<name>' threw:` wrapper (nothing threw — the sandbox refused) nor an `innerMessage`. The existing classifier then does the rest: name is `SandboxError`, no inner/code/fields ⇒ unexpected fault ⇒ `errorFromThrown(err, 500)`, and the message reaching the client is the capability text with the debug prefix stripped. A marker rather than a match on the flattened `SandboxError: …` text, because the flattening is user-reachable: a body that CATCHES the denial and throws its own business error must keep its 400, and that case is pinned. No ADR or contract was changed — this makes the runtime deliver the contract objectstack-ai#3951 already specifies. Tests: new `sandbox/capability-denial-is-a-fault.test.ts` (7) covers all four in-VM gates, the absence of innerMessage/code/fields, the prefix, the caught-and-rethrown rejection, an ordinary deliberate throw, and that a record `ValidationError` crossing `ctx.api` keeps its `code`/`fields` (the marker must not turn every failed write into a 500). Verified failing on all four denial cases before the fix. Runtime suite green: 73 files / 1033 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD * chore: add changeset for the v17 REST envelope defects (objectstack-ai#4431, objectstack-ai#4435, objectstack-ai#4436, objectstack-ai#4483) * fix(test): call syncSchema with its real (object, schema) signature (objectstack-ai#4436) The objectstack-ai#4436 refusal-envelope test passed a single merged object where the driver takes the object name as its own first argument, so the suite could not type-check. Matches the idiom in the sibling memory-driver tests. * fix(metadata-protocol): one probe per PATCH, and the existence gate is not an RLS gate (objectstack-ai#4435) Follow-up to 959b838, fixing two defects the first cut introduced. Both were caught by CI (`Test Core` on @objectstack/objectql, `Dogfood Regression Gate 1/2`), and the second is the more serious of the two. ## 1. The existence probe duplicated OCC's read `updateData` called `assertVersionMatch` (which reads the row for its `updated_at`) and then `assertRecordExists` (which reads the same row again). Two round-trips per PATCH — a performance regression no gate reports — and the `protocol-data.test.ts` OCC cases said so directly ("expected to be called once, but got 2 times"). The two gates want the same row, so they now share one read: `probeRecord` fetches it, `assertVersionOf` became a PURE comparison over an already-read row, and `assertVersionMatch` survives only for `deleteData`, which needs no existence probe at all — the driver's own return reports whether a row matched, so a plain DELETE stays at zero extra reads and only an OCC token buys one. ## 2. The probe must ask EXISTENCE, not the caller's visibility The first cut probed with the CALLER's context, reasoning that it should match `getData`. That quietly turned the existence gate into an authorization gate: a row the caller cannot read comes back `null`, so the PATCH answers 404. Two things break. It moves an RLS decision out of the write policy. Whether an unreadable row may be written by id is the objectstack-ai#1994 pre-image check's call, made inside `engine.update`. A probe in front of it adds a second, different rule — scope creep into the security model, out of a bug fix about missing records. And it disarms a revert-provable security proof. `@proof: rls-by-id-write` (`qa/dogfood/test/rls-fixture.dogfood.test.ts`, referenced by the `permission.rowLevelSecurity.using` liveness ledger entry) boots a fixture whose member can read nothing and has no write policy, and asserts the runner reports `rls-hole` — the RED half that proves the gate can go red at all. A caller-scoped probe 404s that PATCH and the proof goes green: if objectstack-ai#1994 were ever reverted, this probe would MASK it. Accidentally hardening one path is not worth permanently blinding the gate that watches the whole class. So the probe runs as system and answers existence only. Authorization stays exactly where it was, and the sole behaviour added is the 404 the issue asked for: an id that names no row at all. Tests: `protocol-data.test.ts`'s OCC block now asserts the new contract — one probe on every PATCH (the existence probe, no OCC comparison without a token), still exactly one when OCC IS requested (the anti-duplication pin), 404 before any OCC verdict for a missing id, and DELETE without a token issuing no probe. Its fixtures now supply a row, because under this contract a PATCH of an absent record is correctly a 404 and those cases are about OCC. Two cases added to `protocol.record-not-found.test.ts` pin the system-context probe and that an unreadable-but-existing row still reaches the engine for RLS to decide. Green: objectql protocol-data 117, metadata-protocol 170, dogfood shard 1/2 38 files / 235 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4256。
TL;DR
#4226 收口后唯一漏网的 sort 形态补上了:
?sort=account.company_name(点号路径)现在在共享 normalizer 处被拒 ——400 INVALID_SORT,消息说明它想跨的关系、以及正确做法(用公式/汇总字段把值反范式化到本对象再排)。此前它是200+ 未排序行:头段是真实字段所以过闸,然后SqlDriver渲染出"account"."company_name"撞上从未 join 的表,#3821 兜底丢掉排序重跑;Mongo / memory driver 则把路径解析到行内,而外键是标量 id,排序照样无效。sort+top的「最新 N 条」于是变成任意 N 条,响应里没有任何信号。采用 issue 的方案 1(ingress 拒绝),决策依据
Issue 要求先量影响面再定。已对 framework / objectui / cloud 三仓做了穷尽调查:
sys_saved_report种子里全部是平面字段名。accessorKey: fieldName),关联数据走独立的$expand参数嵌在同一平面 key 下;SortBuilder 的字段选项来自objectDef.fields的平面枚举,无自由输入。通用表格的列头排序是纯客户端的,不出网。因此不需要 deprecation 轮次,直接拒绝的爆炸半径就是手写请求 —— 恰好是被静默降级误导的那批调用方。
残余风险(grep 覆盖不到的):客户库里的存量视图元数据。
ObjectView会把工具栏排序持久化进保存视图,sys_saved_report存用户排序;但由于没有任何 UI 路径和种子能写出点号值,存量里存在点号 sort 的唯一途径是管理员在 Studio 元数据管理里手改 JSON(SchemaForm的 repeater 是裸文本输入)。若要绝对确定,可在生产库对保存视图/报表做一次一次性查询。实现
assertSortFieldsExist(packages/metadata-protocol/src/protocol.ts)在原有 unknown-field 检查之后加点号检查,消息按头段区分两种误解:project_id.name)→ 指名它想跨的关系,处方是反范式化(公式/汇总字段);title.length)→ 陈述契约:sort 只作用于整列,不进入列值内部。未知头段(
no_such.title)保持 #4226 的 typo 答案且优先报告 —— 与 expand gate 的unknown>not-a-reference优先级一致。仅注册表有字段表时生效(与所有 #4226 gate 相同的分层);直接调engine.find()的内部调用方不受影响。expand 子查询的嵌套orderBy不经过顶层 gate,无误伤。顺带查证:QueryAST 的显式
joins(点号排序唯一可能合法的场景)已在 #4294 被 tombstone —— 「join 别名排序」这个合法形态不存在。实测(真引擎 + 真 registry,showcase fresh 实例,与 issue 实测表同法)
测试与文档
query-expression-conformance.test.ts新增 12 个用例:七种 wire 拼写的点号拒绝、关系/非关系两种消息、typo 优先级、裸外键控制组、topfootgun 收口。query-syntax.mdx§3 告警改写(「仍静默降级」的前提已不成立)、queries.mdxinfo callout 与data-api.mdx参数表/示例表补点号形态。@objectstack/metadata-protocolpatch。关联
#4256 / #4226(PR #4240)/ #3821 / #3948 / #4294(joins tombstone)
🤖 Generated with Claude Code